fix: prevent disabled models from starting new training rounds - #305

Reviewed against develop in an isolated worktree (branch applies cleanly on top of current tip e9424af, which is also the merge-base; 1 commit, no conflicts — confirmed by both git merge-tree --write-tree locally and GitHub's own mergeable: MERGEABLE / mergeStateStatus: BLOCKED, where BLOCKED is only the missing approvals).

CI on 2a9c546: CI OK green — Solidity (full forge test plus the size gate), Python and Docs. The non-required Integration (full GI) job failed at test_01_platform.py::test_deploy_platform_via_script (missing proxyAdmin in foundry/deployments/localhost.json). That's the same assertion it fails on develop itself (push run on e9424af), so it isn't caused by this PR. It does mean CI never reached this PR's deploy CLI, so I reproduced that locally on anvil. Also reproduced locally: a break-then-fix of the new kill-switch tests, the contract sizes, and the bundled-ABI parity.

Claimed: both startGI overloads reject unregistered or disabled models, including with a funded pool, before any mutation.

Verified — correct, and the tests are load-bearing: _startGI checks state and _GI == GI + 1, then calls dinTaskAuditorContract.validateGIStart(_GI) before updatePassScore, GI++ and _setGIstate. The interface declares it view, so it's a STATICCALL and the auditor can't re-enter. The auditor reverse-looks-up its own address in the registry (TA_ModelNotRegistered), checks modelDisabled(registeredId) (TA_ModelDisabled), then the pool (TC_GIRewardPoolNotFunded, same selector as before). Break-then-fix: the 8 new RewardEngine tests all pass on the PR. With the coordinator line swapped back to develop's if (dinTaskAuditorContract.giRewardPool(_GI) == 0) revert TC_GIRewardPoolNotFunded();, 5 fail ("next call did not revert as expected"): disabled-both-overloads, private-funding-after-disable, unregistered, registry-read-failure, and disabled-in-flight. The 3 that still pass test the auditor directly (constructor wiring, onlyTaskCoordinator, wrong-registry). Restored the file afterwards, and the tree is clean.

Claimed: in-flight rounds can still complete, settle and pay rewards.

Verified — correct: test_disabledInFlight_completesSettlesClaims_butCannotStartNext disables during GI 1, runs it to endGI, claimReward and claimRewards (the balance rises by claimable), then shows both overloads revert for GI 2 with a funded pool. The guard appears only in _startGI; no other entry point changed.

Claimed: the auditor checks the registry-assigned ID, not the constructor's anticipated one.

Verified — correct: the fixture deliberately deploys with constructor ID 1 while the registry assigns ID 0. The test asserts getModelIdByTaskAuditor(ta) == (true, 0), ta.modelId() == 1, and TA_ModelDisabled(0).

Claimed: contract-size gate passes.

Verified — locally: forge build --sizes: DINTaskCoordinator runtime 23,387 B (1,189 B margin), and DINTaskAuditor 21,885 B. Putting the registry reads in the auditor, as Abraham suggested, keeps the coordinator nearly flat. The Solidity CI job enforces the gate too.

Claimed: bundled ABIs match the build artifacts.

Verified — exact: comparing dincli/abis/DINTask{Auditor,Coordinator}.json with foundry/out/… as sorted canonical JSON gives equal ABIs (160/160 and 201/201 entries). The coordinator ABI loses TC_GIRewardPoolNotFunded because that error is now raised by the auditor. dincli prints raw revert text and decodes no custom errors, so nothing in the CLI depends on it.

Claimed: the CLI deploy supports the new constructor shapes.

Verified — on a live anvil, with the real Foundry artifacts: I drove deploy.task_coordinator and deploy.task_auditor themselves. Only the stake/registry lookup, .env writes and sleep were stubbed; transactions went to anvil for real.

tests/test_model_disable_deployment.py: 13 passed locally, and green in CI.

Storage / upgrade safety: both task contracts are non-upgradeable. modelRegistry is immutable (bytecode, not storage), and DINModelRegistry.sol changes only a NatSpec comment, so there is no storage-layout impact on the upgradeable proxy. DINShared.sol adds an interface and two errors only.

Docs: the decisions are recorded with a link to Abraham's comment, which I checked is the real comment ID. The 9 doc files consistently say "future GIs only, no abort, redeploy required, review wiring". Links are green in CI Docs.

Observations:

1. The registry wiring is self-attested (the most important point here, but not a blocker). The only constructor check is modelRegistry_.code.length != 0, and DINModelRegistry.approveModel never reads the auditor's modelRegistry(). A model owner who deploys directly instead of through dincli can pass any contract that returns "registered, enabled". The EnabledModelRegistry test stub is exactly such a contract, and the live deploy above accepted it. That silently defeats the kill switch. The docs say this openly ("Immutable wiring … does not prove the supplied registry is canonical. The DIN-Representative must review…"), and arbitrary custom task code is out of scope anyway. Still, this one check is cheap to make mechanical, in either of two ways:

   • dincli (no contract change): have dinrep approve-registration read taskAuditor.modelRegistry() and refuse unless it equals the platform registry, and likewise in the slasher-authorization path.
   • On-chain: approveModel reverts unless IDINTaskAuditor(req.taskAuditor).modelRegistry() == address(this). That's a DINModelRegistry upgrade, and it would reject legacy pairs.

   I'd suggest a follow-up issue with the dincli check first.

2. deploy task-auditor paired with a coordinator that has no modelId() (a legacy one-argument coordinator, or a wrong --taskCoordinator) crashes with a raw ContractLogicError instead of a BadParameter. Wrapping the modelId().call() in try/except and raising BadParameter("--taskCoordinator does not expose modelId(); …") is a three-line fix at merge.

3. Integration harness: tests/dincli/test_02_task_contracts.py runs model-owner deploy task-coordinator/task-auditor without --model-id. It's already broken on develop, where the CLI passed one constructor argument to a two-argument contract. With this PR it fails earlier, with a clear BadParameter. It needs --model-id in the task_061026_21 Part D rework of the suite. Not this PR's job, but noting it so it isn't missed.

4. Merge order with PR No. 261: confirmed. git merge-tree of this branch against No. 261's head conflicts in dincli/cli/modelownerd/deploy.py, since both rewrite the constructor wiring. cache_model_0/abis/ (refreshed by No. 261) also isn't touched here, so the reference model's bundled task ABIs stay stale until No. 261 lands. As the author says: No. 261 first, then rebase this one and re-run.

Not independently re-verified: the full forge test (524 passed per the PR; covered by the green CI Solidity job) and a full GI through dincli + Docker/Kubo (the integration suite is red on develop for an unrelated reason).

Every checkable claim held up, and the kill-switch tests demonstrably guard the change. The contract side is mergeable as-is. Before merge: land PR No. 261 first and rebase (the deploy.py conflict), and ideally apply observation 2. Observation 1 should become a follow-up issue rather than block this PR, since the docs already scope the guarantee to reviewed wiring. This touches foundry/src/** and implements a mechanism decision, so per MAINTAINING No. 4.2 it also needs @abrahamnash's sign-off as the second key alongside mine. The PR says "Addresses issue No. 224" (no closing keyword). That's right for now: all three decisions in No. 224 are covered here, so No. 224 can go through din-close-issue after merge, with observation 1 as its follow-up.

Files changed (31) — as of 2a9c546 (PR head)

Diffed against merge-base e9424af (develop). develop hasn't moved since the branch was cut. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (approvals only). Confirmed with a local git merge-tree --write-tree dry run: clean. Against open PR No. 261's head it conflicts in dincli/cli/modelownerd/deploy.py (and in Developer/BACK_LOG.md, which comes from No. 261 alone). Clean against PRs No. 303 and No. 304.

foundry/src/DINTaskAuditor.sol

foundry/src/DINTaskCoordinator.sol

foundry/src/DINShared.sol, foundry/src/DINModelRegistry.sol

dincli/cli/modelownerd/deploy.py

dincli/abis/DINTaskAuditor.json, dincli/abis/DINTaskCoordinator.json

tests/test_model_disable_deployment.py

foundry/test/RewardEngine.t.sol

foundry/test/utils/EnabledModelRegistry.sol + constructor updates in 12 test files

AggregatorCommitReveal, AuditorCommitReveal, DeployPlatform, DisputeResolution, EncryptedTestData, GasSimulation, LifecycleEvents, PR146SlashingRegression, ScoringValidation, SecurityFindings, StakingEnforcement, TreasuryForwarding (.t.sol)

Documentation (9 files)

public/roles/dinrep.md, public/workflows/model-workflow.md, technical/audits/foundry-src-security-review.md, technical/contracts/{DINModelRegistry,DINShared,DINTaskAuditor,DINTaskCoordinator,DinValidatorStake}.md, technical/mechanisms/staking-mechanism.md

Verification

CI on 2a9c546: CI OK green. The non-required Integration (full GI) job is red on the same pre-existing missing proxyAdmin assertion as develop. Locally (real via_ir profile, after npm ci): forge build --sizes, the 8 kill-switch tests (8/8), break-then-fix (5 red without the coordinator call), the ABI parity check, pytest tests/test_model_disable_deployment.py (13 passed), and a live anvil deploy of the real Foundry task pair through the PR's deploy.py commands. Full detail is in the verification comment above.

Approvals needed (MAINTAINING No. 4.2): foundry/src/** plus a mechanism decision: the peer approval (@umeradl) and @abrahamnash as the second key.

Local vs. GitHub agree: both clean against develop.

Thanks for the detailed review and deployment checks, @umeradl!

Pushed the focused error handling in dfc5bd290644335ffd883cecec923735d45fcf16: a revert when reading the paired coordinator's modelId() now produces a clear BadParameter with guidance to check the address and compatibility. Unrelated errors still propagate.

All 24 targeted deployment tests passed in an isolated environment, covering the failure paths, absence of deployment side effects, and legacy compatibility. I haven't rerun live deployment or full-GI tests.

I'll keep this waiting for PR No. 261, then reconcile the deployment overlap and rerun the relevant checks. Abraham's requested sign-off remains outstanding, and I'll keep registry-wiring enforcement as a separate follow-up. Fresh CI and re-review of the new head are still needed.

Reviewed against develop in an isolated worktree (pr-305-review on top of 90a72422, the PR head). Peer check: OK (Santiagocetran is the author; operator is not).

CI on 90a72422: CI OK ✅, Python ✅ (all non-integration tests pass), Solidity ✅, Docs ✅ — all green. Review is based on source inspection and CI.

Claimed — validateGIStart blocks both startGI overloads when the model is disabled. Verified: DINTaskCoordinator._startGI (L370–385) is the shared internal path called by both startGI(uint _GI, uint score) (L360) and startGI(uint _GI) (L366). Line 381 calls dinTaskAuditorContract.validateGIStart(_GI) unconditionally before any GI state change. Neither public overload touches state before that call.

Claimed — validateGIStart checks registry approval, disabled status, and reward pool in order. Verified by direct inspection of DINTaskAuditor.sol L446–451: getModelIdByTaskAuditor(address(this)) first; TA_ModelNotRegistered if !exists; TA_ModelDisabled(registeredId) if modelDisabled; TC_GIRewardPoolNotFunded if giRewardPool[gi] == 0. The implementation uses the registry-returned registeredId (the canonical ID from the registry), not the constructor's modelId (the stake-floor parameter). These two can differ if the registry assigns a different ID than the one passed to the constructor; the PR uses the authoritative registry value, which is correct.

Claimed — ITaskModelRegistry interface matches the real DINModelRegistry functions. Verified against DINModelRegistry.sol: getModelIdByTaskAuditor(address) is at L422–429, returns (bool exists, uint256 modelId) using a val - 1 sentinel to distinguish model ID 0 from "not registered" — exactly what the interface declares. modelDisabled(uint256) is the auto-getter for the public modelDisabled mapping at L119. Both signatures match the interface in DINShared.sol.

Claimed — modelRegistry is immutable and validated as a contract at construction. Verified: declared ITaskModelRegistry public immutable modelRegistry at L22; constructor checks modelRegistry_.code.length == 0 (L419) and reverts TA_InvalidAddress before assignment (L428). No setter exists; cannot be reassigned after deployment.

Claimed — all three errors are file-level selectors, not contract-scoped. Verified: TC_GIRewardPoolNotFunded (L359), TA_ModelNotRegistered (L488), and TA_ModelDisabled(uint256) (L490) are all in DINShared.sol. The pre-PR coordinator used TC_GIRewardPoolNotFunded as a file-level error; the selector is unchanged.

Claimed — 8 new kill-switch tests in RewardEngine.t.sol. Verified by CI (Solidity ✅) and by inspection: 8 new tests including test_disabledModel_blocksBothOverloadsAtomically_andReenable (both startGI overloads blocked, state unchanged, re-enable resumes) and test_disabledInFlight_completesSettlesClaims_butCannotStartNext (in-flight GI completes normally; next GI blocked after disable). EnabledModelRegistry.sol test stub always returns (true, 0) / false and is used in all non-kill-switch tests — this keeps existing test suites unaffected by the new auditor constructor arg.

Claimed — SecurityFindings.t.sol constructors updated. Confirmed: the existing security-finding regression tests now pass the 4-arg auditor constructor (CI Solidity ✅).

Claimed — dincli model-owner deploy handles the new 4-arg auditor constructor. Verified: _task_constructor_args in dincli/cli/modelownerd/deploy.py dispatches on constructor arity; 4-arg auditor passes [stake, coordinator, model_id, registry]. test_supported_constructor_shapes in test_model_disable_deployment.py exercises all five supported shapes including ("address", "address", "uint256", "address") → ["stake", "coordinator", 0, "registry"]; Python CI green.

Claimed — DINTaskAuditor.json ABI updated. Verified: new constructor input modelRegistry_, modelRegistry view function, validateGIStart function, TA_ModelNotRegistered and TA_ModelDisabled errors all present.

Claimed — security review M-2 updated with resolution. Verified in foundry-src-security-review.md: "Resolution" paragraph added above the original finding text; L-4 updated to "Confirmed intentional — [#224, Oct 26 decision] / BL-35." The original finding text is retained as audit context with the label "Original finding — ... superseded for new compatible pairs."

Claimed — full-gi.yml extracted from ci.yml. Verified: 178 lines removed from ci.yml (the integration-paths + integration jobs); equivalent job added as full-gi.yml with on: workflow_dispatch (manual only). The ci-ok job in ci.yml never waited on integration, so this is a no-op for required checks.

Finding — P2: Developer/BACK_LOG.md BL-34 status not updated to resolved. The PR changes BL-34 from "🆕 New — unaddressed" to "🆕 Decided, unimplemented — Abraham (Oct 26, [No. 224]): make the kill-switch real…" — but this PR IS that implementation. BL-34 should be "✅ Resolved — validateGIStart wired in DINTaskAuditor.sol; both startGI overloads check registry approval and disabled status before any state change. Compatible task pairs required; existing deployed pairs unaffected. PR No. 305."

Merge state: branch is BEHIND develop (GitHub mergeStateStatus: BEHIND), no conflict (local merge-tree exit 0). Needs update-branch before merge. foundry/src/** touched — a second sign-off from @umeradl or @abrahamnash is required per MAINTAINING No. 4.2 before this merges.

Head at review: 90a72422
Merge base (upstream develop at PR creation): earlier develop tip
Current upstream develop: behind but clean — no conflict (merge-tree exit 0)

foundry/src/DINShared.sol

foundry/src/DINTaskAuditor.sol

foundry/src/DINTaskCoordinator.sol

foundry/test/utils/EnabledModelRegistry.sol (new)

foundry/test/RewardEngine.t.sol

foundry/test/SecurityFindings.t.sol

dincli/cli/modelownerd/deploy.py

dincli/abis/DINTaskAuditor.json

tests/test_model_disable_deployment.py (new)

Developer/BACK_LOG.md

Documentation/technical/audits/foundry-src-security-review.md

Documentation/technical/contracts/DINTaskAuditor.md

Documentation/technical/contracts/DINTaskCoordinator.md, DINShared.md, DINModelRegistry.md, DinValidatorStake.md

.github/workflows/ci.yml / .github/workflows/full-gi.yml (new)

Remaining documentation files (skills, MAINTAINING.md, CONTRIBUTING.md, ROADMAP.md, role/workflow docs, README)

Thanks for the reviews and thorough checks!

Fixed BL-34 in 6970f5d to mark it as resolved and describe the implemented guard, including the compatibility caveat for existing deployments.

Only the backlog status changed. Diff checks and all 293 documentation-link checks passed.

I’ll reconcile the deployment changes once PR No. 261 lands, then rerun the relevant checks and request a fresh review.

Thanks for the thorough review! I’ve reconciled this branch with develop after PR No. 261 and pushed the updates through e8e7349.

The CLI now checks the coordinator’s protocol version before deploying or pairing an auditor, rejecting incompatible combinations while preserving matching current and legacy pairs. The artifacts and docs are updated too.

Local validation is green: 735 Python tests and 527 Foundry tests passed. Live CLI checks confirmed that all eight incompatible pairings reject without transactions or address writes. Astra also rechecked the fix and found no new issues.

Fresh CI is running. Persistent IPFS publication and independent retrieval, along with fresh CI results and the required reviews, are still pending before this is ready to merge. The current IPFS evidence covers local offline pinning and retrieval only.

Thanks again for the thorough reviews! PR #261 is now incorporated, the deployment changes are reconciled, and the earlier requested fixes are addressed. Latest CI is green: 527 Foundry tests and 895 Python tests passed, including artifact parity and contract-size checks.
@umeradl @robertocarlous

Reconciled PR #305 with the latest develop, resolved the ABI conflict, and regenerated the task artifacts and manifest CIDs. Pushed in 2d9f5c9. GitHub now reports the branch as mergeable.

Fresh validation passed:

• 534 Foundry tests
• 895 Python tests
• 32 Hardhat tests
• Artifact parity, contract-size gate, and all 298 documentation links
• 27 live checks on a separate device, covering real CLI deployment/pairing, incompatible-pair rejection, and both GI-start overloads blocking unregistered or disabled models and succeeding after re-enabling

Fresh GitHub CI is queued. Passing CI and the required peer review/sign-offs remain before merge.

Re-review of the fix commits: 2a9c546 (reviewed 2026-10-30) → f46dd02 (current HEAD). These are the 6 non-merge commits since then (dfc5bd2, 6970f5d, 33d5f83, e8e7349, 7d20067, c8a7224) plus the develop reconcile merges. Reviewed in the same isolated worktree, hard-reset to f46dd02. develop's tip ad94870 is also the merge-base, so the branch is fully up to date. git merge-tree --write-tree against origin/develop is clean (exit 0), and GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (approvals only). Peer check: OK.

CI on f46dd02: CI OK green: Solidity (full forge test, size gate, and the new artifact-bundle gate), Python and Docs. Reproduced locally (CI doesn't cover these): the deploy CLI's new generation check on a live anvil, bytecode reproducibility of the bundled artifacts, contract sizes, and the manifest CIDs.

What changed in the PR's own diff since 2a9c546: 23 of the 42 files are byte-for-byte the same change I reviewed (this includes DINTaskAuditor.sol, DINShared.sol and DINModelRegistry.sol). The contract delta is a single pure getter, DINTaskCoordinator.taskProtocolVersion() == 1. Everything else is in deploy.py, the bundled artifacts, CI, tests and docs.

Claimed: a revert reading the paired coordinator's modelId() now gives a clear BadParameter (dfc5bd2, my observation No. 2).

Verified — fixed: ContractLogicError/BadFunctionCallOutput are wrapped; ConnectionError/ValueError still propagate (test_auditor_command_preserves_unrelated_read_errors). On anvil, an EOA passed as --taskCoordinator gives BadParameter: Cannot read modelId() from the paired coordinator; verify the address… and sends 0 tx.

Claimed: the CLI rejects incompatible coordinator/auditor generations before the one-shot pairing (e8e7349).

Verified — on a live anvil, with real artifacts: I drove dincli's own deploy.task_coordinator and deploy.task_auditor; only din_info, .env writes and sleep were stubbed. "Develop" below means develop's bundled cache_model_0/abis artifacts, i.e. the generation before this PR:

Both rejected rows matter. Row 2 would brick the pair: the new coordinator calls validateGIStart, which the old auditor doesn't have, so every startGI would revert. Row 3 would silently drop the kill switch: the old coordinator never calls the guard. test_incompatible_generations_send_no_transaction and test_missing_version_getter_only_allows_legacy_pairs cover the same matrix with mocks.

Claimed: BL-34 is marked resolved (6970f5d, Robbert's P2).

Verified: the BL-34 status cell is now Robbert's suggested text, verbatim. BL-31 and BL-35 also record what this PR documents.

Claimed: PR No. 261 is reconciled, and the reference-model artifacts and manifest CIDs are refreshed (my observation No. 4).

Verified — exact: after a local forge build (real via_ir profile), python3 .github/scripts/check_task_artifacts.py prints "Task ABI bundles and reference creation bytecode match the Foundry build". The committed creation bytecode reproduces byte-for-byte on a second machine, not just on CI. I recomputed both manifest CIDs as raw CIDv1 (sha2-256) of the committed files and both match: bafkreigjv7cx… for DINTaskCoordinator.json (103,902 B) and bafkreidgp65w… for DINTaskAuditor.json (102,120 B). I could not confirm they're pinned: public gateways rate-limited or timed out.

Claimed: the two pinned remappings make the bytecode reproducible (c8a7224).

Verified, indirectly: with the pins in place, my local build matches the committed bytecode exactly, as above. Neither added remapping (erc4626-tests/, halmos-cheatcodes/) is imported by foundry/src. They only pin what forge's auto-detection would otherwise choose between duplicate nested libs, and that choice shows up in the metadata hash.

Claimed: contract-size gate passes.

Verified — locally: DINTaskCoordinator runtime is 23,321 B (1,255 B margin, warn tier, above the 1,024 B fail line). It was 23,387 B at 2a9c546: develop shrank it in the meantime, and the new getter costs very little. DINTaskAuditor is 21,887 B (2,689 B margin).

Claimed: the kill-switch tests still guard the change, plus a new disabled-status read-failure test.

Verified: forge test --match-contract RewardEngine: 40 passed locally. The new test_disabledStatusReadFailure_failsClosed covers a registry whose modelDisabled reverts. test_registryWiring_rejectsZeroAndEOA also asserts taskProtocolVersion() == 1. The break-then-fix from my first review still applies, because the coordinator's validateGIStart line is unchanged.

Previous observations — status:

1. Self-attested registry wiring — still true by design (the auditor's constructor only checks modelRegistry_.code.length), and no follow-up issue exists yet. Issue No. 185 (DIN-Rep approval automation) is the natural home, as two checks before approveModel/add-slasher: taskAuditor.modelRegistry() == <platform registry> and taskCoordinator.taskProtocolVersion() == 1. I can add that there.
2. Raw ContractLogicError — fixed, see above.
3. Integration test needs --model-id — moot now: since PR No. 261 the coordinator defaults to totalModels() and the auditor to the coordinator's modelId(). Full GI is manual-only and I didn't run it.
4. Merge order with No. 261 — done. No. 261 landed and was reconciled, and cache_model_0 ABIs are now refreshed and gated.

New observations (none blocking):

1. The new bytecode-parity gate is a standing cost for every future task-contract PR. check_task_artifacts.py requires cache_model_0/abis/*.json creation bytecode to equal the build exactly, and it runs in the required Solidity job. Because the metadata hash is part of the bytecode, any edit to DINTaskCoordinator.sol, DINTaskAuditor.sol, DINShared.sol or an OZ submodule now fails CI until both bundles are regenerated, even a NatSpec-only edit. The two manifest CIDs then have to be updated and re-pinned too. Two task-contract PRs open at the same time will always conflict in these three files. Two small changes would make that manageable:

   • (a) The failure message should name the regen command. I checked that dincli system dump-abi --artifact foundry/out/<C>.sol/<C>.json --output cache_model_0/abis --bytecode reproduces both committed files byte-for-byte.
   • (b) The gate should also check the manifest's two ipfs CIDs against the files. Right now a refreshed bundle with a stale CID passes, which is exactly the drift this gate is meant to stop. The raw-CIDv1 computation is a few lines of stdlib (0x01 0x55 0x12 0x20 + sha256, base32).

   Whether cache_model_0 should keep exact-bytecode parity at all, rather than ABI-only, is a maintainer call. I'm flagging it rather than asking for it here.

2. Unreachable "no deployed code" message: in deploy task-auditor, the w3.eth.get_code check runs after the modelId() read. So for the 3/4-arg auditors, an address without code always hits the modelId() message instead (the anvil EOA row above). Moving the get_code check above the modelId() read gives the clearer error. Two-line move.

3. Please confirm the two new bafkrei… CIDs are pinned on the project's IPFS provider. Nothing in CI fetches them.

Not independently re-verified: the full forge test (527 per the author; covered by the green Solidity job), the full non-integration pytest (covered by the green Python job), and a full GI through dincli + Docker/Kubo (full-gi.yml is manual-only).

Every new claim held up. The contract change since my approval is one pure getter, and the CLI generation check behaves correctly on a live chain in all four pairings. Mergeable as-is. New observations 1(a), 2 and 3 are small and can go in before merge or at merge; 1(b) and the exact-bytecode question can be a follow-up. My earlier approval was dismissed by the new commits; a fresh first-key approval follows separately. This touches foundry/src/** and implements the No. 224 mechanism decision, so it still needs @abrahamnash's approval as the second key (MAINTAINING No. 4.2). @robertocarlous, your P2 is applied verbatim at 6970f5d. Could you re-review or dismiss your change request? The PR body has no closing keyword. That's still right: No. 224 goes through din-close-issue after merge, with previous observation 1 as its follow-up.

Files changed (42) — as of f46dd02 (PR head), re-review delta vs 2a9c546

Diffed against merge-base ad94870, which is the current develop tip, so the branch is fully up to date. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (approvals only). A local git merge-tree --write-tree dry run is clean.

23 files are unchanged since my first review: the PR's own change to each is identical to 2a9c546, so their rows in the first merge-proposal comment stand. They are foundry/src/DINTaskAuditor.sol, DINShared.sol, DINModelRegistry.sol, foundry/test/utils/EnabledModelRegistry.sol, the 12 constructor-arg test updates (AggregatorCommitReveal, AuditorCommitReveal, DeployPlatform, DisputeResolution, EncryptedTestData, GasSimulation, LifecycleEvents, PR146SlashingRegression, ScoringValidation, SecurityFindings, StakingEnforcement, TreasuryForwarding), dincli/abis/DINTaskAuditor.json, and the docs DINModelRegistry.md, DINShared.md, DinValidatorStake.md, staking-mechanism.md, dinrep.md and foundry-src-security-review.md. The 19 files that changed or are new follow.

foundry/src/DINTaskCoordinator.sol

dincli/cli/modelownerd/deploy.py

.github/scripts/check_task_artifacts.py (new), .github/workflows/ci.yml, tests/test_task_artifact_check.py (new)

cache_model_0/abis/DINTaskCoordinator.json, cache_model_0/abis/DINTaskAuditor.json, cache_model_0/manifest.json

dincli/abis/DINTaskCoordinator.json

foundry/remappings.txt

foundry/test/RewardEngine.t.sol

tests/test_model_disable_deployment.py, tests/test_deploy_task_contracts.py, tests/test_task_auditor_model_id_guard.py

Developer/BACK_LOG.md

Documentation/public/workflows/model-workflow.md, Documentation/public/roles/model-owner.md, Documentation/technical/contracts/DINTaskAuditor.md, Documentation/technical/contracts/DINTaskCoordinator.md

Verification

See the re-review verification comment above: live-anvil pairing matrix (4 pairings plus the EOA case), local via_ir build with the artifact gate passing, RewardEngine 40/40, the size gate, and the manifest CIDs recomputed. Everything else is from CI run 42274091689 on f46dd02.

Local vs. GitHub agree: yes, clean merge in both, branch up to date with develop.

Actual outcome — PR #305 merged as-is (pushed)

This supersedes the pre-merge review comment above with what actually happened.

One commit on origin/develop:

1. 3def72f — real merge of this PR (merge commit, not squash) through the merge queue. Its parents are ad94870 (develop) and f46dd02 (the PR head, the SHA I re-reviewed and approved). There were no conflicts, as predicted. GitHub agrees: PR shows MERGED, merged by @robertocarlous at 14:43 UTC.

No deviation commit was needed.

Sign-offs: @robertocarlous as peer (approved at f46dd02, 14:36 UTC, which also cleared his Oct 31 change request), and @umeradl as the foundry/src/** second key and mechanism sign-off (approved at f46dd02, 12:17 UTC). That's what MAINTAINING No. 4.2 requires for a PR not authored by Umer. My re-review comment also asked for @abrahamnash as second key, which went beyond what No. 4.2 requires. The mechanism sign-off section says a PR doesn't wait for Abraham on GitHub, so his approval wasn't needed here.

Files unchanged from the PR (42 of 42)

git diff f46dd02 3def72f over the PR's 42 files is empty, so every file landed exactly as authored. None of them has been touched on develop since.

• Contracts: foundry/src/DINTaskCoordinator.sol, DINTaskAuditor.sol, DINShared.sol, DINModelRegistry.sol, foundry/remappings.txt
• Contract tests: 14 files under foundry/test/ (including the new utils/EnabledModelRegistry.sol)
• Artifacts: cache_model_0/abis/DINTaskCoordinator.json, DINTaskAuditor.json, cache_model_0/manifest.json, dincli/abis/DINTaskCoordinator.json, DINTaskAuditor.json
• CLI and CI: dincli/cli/modelownerd/deploy.py, .github/scripts/check_task_artifacts.py (new), .github/workflows/ci.yml
• Python tests: tests/test_model_disable_deployment.py (new), test_task_artifact_check.py (new), test_deploy_task_contracts.py, test_task_auditor_model_id_guard.py
• Docs: Developer/BACK_LOG.md, Documentation/public/roles/{dinrep,model-owner}.md, Documentation/public/workflows/model-workflow.md, five Documentation/technical/contracts/*.md files, mechanisms/staking-mechanism.md, audits/foundry-src-security-review.md

Not done before merge (follow-ups)

The non-blocking observations from the re-review weren't applied before merge (the head stayed at f46dd02), so they're still open:

• No. 1(a): check_task_artifacts.py's bytecode-mismatch message should name the regeneration command (dincli system dump-abi --artifact foundry/out/<C>.sol/<C>.json --output cache_model_0/abis --bytecode).
• No. 1(b): the gate should also check cache_model_0/manifest.json's two ipfs CIDs against the files. Whether cache_model_0 should keep exact-bytecode parity at all is a maintainer call.
• No. 2: in deploy task-auditor, move the w3.eth.get_code check above the modelId() read so that an address with no code gets the "no deployed code" message.
• No. 3: confirm the two new bafkrei… CIDs are pinned on the project's IPFS provider.

Issue No. 224 goes through din-close-issue next, with these as its follow-up list.

Verification

The merge-queue run on the merge-group commit and the post-merge push run on 3def72f are both green: Python, Solidity (including the new task-artifact gate), Docs and CI OK. CodeQL (Code Quality) also passed. The full GI through dincli + Docker/Kubo wasn't run, because full-gi.yml is manual-only.

Local vs. GitHub agree: clean merge, as both predicted.